Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pull] main from microsoft:main #171

Merged
merged 4 commits into from
May 7, 2024
Merged

[pull] main from microsoft:main #171

merged 4 commits into from
May 7, 2024

Conversation

pull[bot]
Copy link

@pull pull bot commented May 7, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

lhecker and others added 4 commits May 6, 2024 18:20
This fixes:
* `HRESULT`s not being shown as unsigned hex
* `D2DERR_RECREATE_TARGET` not being handled
* 4 calls not checking their `HRESULT` return
  Out of the 4 only `CreateCompatibleRenderTarget` will throw in
  practice, however it throws `D2DERR_RECREATE_TARGET` which is common.
  Without this error handling, AtlasEngine may crash.

## Validation Steps Performed
* Set Graphics API to Direct2D
* Use `DXGIAdapterRemovalSupportTest.exe` to trigger
  `D2DERR_RECREATE_TARGET`
* No error message is shown ✅
* If the `D2DERR_RECREATE_TARGET` handling is removed, the application
  never crashes due to `cursorRenderTarget` being `nullptr` ✅
When the VT render engine starts a paint operation, it first checks to
see whether there is actually something to do, and if not it can end the
frame early. However, the result of that check was being ignored, which
could sometimes result in an unwanted `SGR` reset being written to the
conpty pipe.

This was particular concerning when passing through `DCS` sequences,
because an unexpected `SGR` in the middle of the `DCS` string would
cause it to abort early.

This PR addresses the problem by making sure the `VtEngine::StartPaint`
return value is appropriately handled in the `XtermEngine` class.

## Detailed Description of the Pull Request / Additional comments

To make this work, I also needed to correct the `_cursorMoved` flag,
because that is one of things that determines whether a paint is needed
or not, but it was being set in the `InvalidateCursor` method at the
start of ever frame, regardless of whether the cursor had actually
moved.

I also took this opportunity to get rid of the `_WillWriteSingleChar`
method and the `_quickReturn` flag, which have been mostly obsolete for
a long time now. The only place the flag was still used was to optimize
single char writes when line renditions are active. But that could more
easily be handled by testing the `_invalidMap` directly.

## Validation Steps Performed

I've confirmed that the test case in issue #17117 is no longer aborting
the `DCS` color table sequence early.

Closes #17117
When the `DCS` passthrough code was first implemented, it relied on the
`ActionPassThroughString` method flushing the given string immediately.
However, that has since stopped being the case, so `DCS` operations end
up being delayed until the entire sequence has been parsed.

This PR fixes the issue by introducing a `flush` parameter to force an
immediate flush on the `ActionPassThroughString` method, as well as the
`XtermEngine::WriteTerminalW` method that it calls.

## Validation Steps Performed

I've confirmed that the test case in issue #17111 now updates the color
table as soon as each color entry is parsed, instead of delaying the
updates until the end of the sequence.

Closes #17111
@pull pull bot added the ⤵️ pull label May 7, 2024
@pull pull bot merged commit 9c16c5c into l3dlp-sandbox:main May 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants